feat(extensions): add GitHub handler extension and env var access#62
Conversation
- add github-handler example extension that runs Kit as a GitHub collaborator inside Actions: parses the event, gates on author_association, drives the agent, posts comments, and opens PRs - seed the Yaegi interpreter with os.Environ() in the loader and test harness so extensions can read env vars (e.g. GITHUB_EVENT_PATH) via os.Getenv/LookupEnv/Environ without mutating the host environment - document env var access, the new extension, and env-aware testing across the docs site, README, and kit-extensions skill Part of #60
|
Connected to Huly®: KIT-63 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a new ChangesGitHub Handler Extension and Environment Seeding
Sequence Diagram(s)sequenceDiagram
participant GitHubActions as GitHub Actions
participant OnSessionStart as OnSessionStart
participant gh as gh CLI
participant KitAgent as Kit Agent
participant OnAgentEnd as OnAgentEnd
GitHubActions->>OnSessionStart: trigger (GITHUB_EVENT_PATH)
OnSessionStart->>OnSessionStart: loadEvent → buildTrigger → auth check
OnSessionStart->>gh: addReaction("eyes")
rect rgba(200, 200, 255, 0.5)
Note over OnSessionStart,gh: when not dry-run and gh exists
OnSessionStart->>gh: fetch PR diffs and comment threads
end
OnSessionStart->>OnSessionStart: buildPrompt
OnSessionStart->>KitAgent: DriveAgent(prompt)
KitAgent->>OnAgentEnd: AgentEnd(response)
OnAgentEnd->>gh: postComment(response or error)
alt uncommitted changes exist
OnAgentEnd->>OnAgentEnd: git commit as kit-agent[bot]
OnAgentEnd->>gh: git push + gh pr create
end
OnAgentEnd->>gh: addReaction("rocket" or "confused")
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
examples/extensions/github-handler/main.go (1)
141-143: ⚡ Quick winUse session-scoped trigger state instead of a single package-global pointer.
activeTriggeris a single mutable slot shared across callbacks. If sessions overlap or callbacks interleave, replies/PR side effects can be associated with the wrong trigger. Store trigger state by session (with synchronization) and clear it after completion.As per coding guidelines, "Widget/header/footer/editor state lives on the Runner with
sync.RWMutexfor thread safety, queried by UI via callbacks" and "Extension contexts use function fields (Print func(string),SetWidget func(WidgetConfig)) wired by closures incmd/root.gofor state management".Also applies to: 184-184, 346-347
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/extensions/github-handler/main.go` around lines 141 - 143, The package-global `activeTrigger` pointer is a single mutable slot shared across all sessions and callbacks, creating a race condition when sessions overlap or callbacks interleave. Replace this single package-level variable with a session-scoped state management approach: use a map keyed by session identifiers (protected by sync.RWMutex for thread safety) to store trigger state separately for each session, following the pattern established in your coding guidelines where state lives on the Runner with proper synchronization. At examples/extensions/github-handler/main.go line 141-143, replace the `var activeTrigger *trigger` declaration with a protected map structure; at line 184-184, update the code that writes to activeTrigger to instead write to the session-keyed map entry; and at line 346-347, update the code that reads from activeTrigger to instead read from the session-keyed map entry. Ensure the trigger state is cleared after session completion to prevent memory leaks and state pollution.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/extensions/github-handler/main.go`:
- Around line 449-473: The gitOutput, ghOutput, and runCmd functions use
unbounded exec.Command calls that can hang indefinitely if the git or gh
commands stall due to network issues, authentication prompts, or other blocking
conditions. To fix this, modify all three functions to use exec.CommandContext
instead of exec.Command, passing a context with an appropriate timeout (such as
a context derived from the provided ctx parameter with a reasonable duration
like 30 seconds). Ensure the timeout is applied consistently across gitOutput,
ghOutput, and runCmd to prevent workflow stalls when subprocess calls become
unresponsive.
- Around line 275-279: The extractRequest function incorrectly recognizes the
commandToken when it appears in the middle of prose with spaces around it (like
"please review /kit behavior"). Remove or modify the strings.Index check that
looks for " "+commandToken+" " anywhere in the trimmed string, as this matches
incidental mentions. Instead, only recognize the command when it appears at the
beginning of the message (using strings.HasPrefix with commandToken+" ") or at
the end (the existing strings.HasSuffix case). This ensures the handler only
triggers on actual command invocations, not mid-sentence prose containing the
token.
---
Nitpick comments:
In `@examples/extensions/github-handler/main.go`:
- Around line 141-143: The package-global `activeTrigger` pointer is a single
mutable slot shared across all sessions and callbacks, creating a race condition
when sessions overlap or callbacks interleave. Replace this single package-level
variable with a session-scoped state management approach: use a map keyed by
session identifiers (protected by sync.RWMutex for thread safety) to store
trigger state separately for each session, following the pattern established in
your coding guidelines where state lives on the Runner with proper
synchronization. At examples/extensions/github-handler/main.go line 141-143,
replace the `var activeTrigger *trigger` declaration with a protected map
structure; at line 184-184, update the code that writes to activeTrigger to
instead write to the session-keyed map entry; and at line 346-347, update the
code that reads from activeTrigger to instead read from the session-keyed map
entry. Ensure the trigger state is cleared after session completion to prevent
memory leaks and state pollution.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2e104fd3-93de-4a15-8f6a-c0cba357687a
📒 Files selected for processing (11)
README.mdexamples/extensions/README.mdexamples/extensions/github-handler/README.mdexamples/extensions/github-handler/main.goexamples/extensions/github-handler/main_test.gointernal/extensions/loader.gopkg/extensions/test/harness.gowww/pages/cli/commands.mdwww/pages/extensions/examples.mdwww/pages/extensions/loading.mdwww/pages/extensions/testing.md
- only trigger on /kit at the start or end of a comment line, ignoring incidental mid-sentence mentions like "please review /kit behavior" - bound git/gh subprocess calls with a 30s timeout via CommandContext so a stalled network call or auth prompt cannot hang the Actions job - add a regression test for the mid-sentence mention case Part of #60
Description
This adds the GitHub handler — the runtime half of Kit's GitHub integration (Phase 2b of #60) — plus the small extension-runtime change it depends on.
The new
examples/extensions/github-handlerextension is designed to run inside a GitHub Actions runner, driven by the workflow thatkit github installscaffolds. When a collaborator comments/kit <request>on an issue or PR, it parses the event fromGITHUB_EVENT_PATH, enforces that the author has write/admin access (author_associationinOWNER/MEMBER/COLLABORATOR), reacts 👀, gathers context (issue thread or PR diff viagh), drives the agent, posts the response back as a comment, and — if the agent left uncommitted changes — pushes akit-agent[bot]branch and opens a PR. It is inert outside Actions (GITHUB_ACTIONS != "true"), so it is safe to keep loaded locally, and aKIT_GITHUB_DRY_RUNmode logs allgh/gitside effects instead of executing them for deterministic testing.Building this surfaced a prerequisite: Yaegi runs extensions in restricted mode where
os.Getenv/os.LookupEnv/os.Environread a virtualized environment that Kit left empty, so extensions could not read any env var. The interpreter (in both the production loader and the test harness) is now seeded withos.Environ(). This is read-only from the host's perspective —os.Setenvonly mutates the extension's sandboxed copy — and is strictly weaker than theos/execaccess extensions already have.Type of Change
Checklist
Additional Information
Part of #60 (this is one phase of the GitHub-integration epic, so it intentionally does not auto-close the umbrella issue).
New files
examples/extensions/github-handler/main.go— the handler extension (OnSessionStartparse/gate/drive,OnAgentEndcomment + PR)examples/extensions/github-handler/main_test.go— 8 tests (permission gate,/kitparsing, issue vs PR-review-comment paths, comment posting, PR-on-dirty-tree) using dry-run modeexamples/extensions/github-handler/README.md— usage, env vars, options, security notesModified
internal/extensions/loader.go,pkg/extensions/test/harness.go— seed the interpreter env withos.Environ()www/pages/extensions/loading.md— new "Standard library access" section documenting env-var readability and sandboxingwww/pages/extensions/testing.md— testing extensions that read env vars (t.Setenvbefore load)www/pages/extensions/examples.md,www/pages/cli/commands.md,README.md,examples/extensions/README.md— index the extension and cross-reference it from the GitHub-integration docsBackward compatibility: no breaking changes. Extensions that never read env vars are unaffected; those that previously got empty strings from
os.Getenvnow receive real values.Verification:
go test -race ./internal/extensions/... ./pkg/extensions/... ./examples/extensions/...andgo test ./cmd/...pass;go vet ./...clean;golangci-lint runreports 0 issues; thewwwdocs site builds cleanly.Summary by CodeRabbit
Release Notes
/kittriggers in issue comments and pull request review comments.